Repository navigation
docs: clarify that supportingFiles is an allow-list (#22238) - #25075
brbousnguar wants to merge 2 commits into
Conversation
OpenAPITools#22238 reports that the restclient ApiClient references ServerConfiguration, ServerVariable and ExceptionProvider without those classes being available. They are registered as supporting files and live in the same package as ApiClient, so a default run emits all of them and there is nothing to import. The reported build fails because its <supportingFilesToGenerate> list predates the ApiClient gaining those references, and DefaultGenerator treats the list as a fixed allow-list, so the three files are skipped. Document that trap where it is read: a paragraph in the Selective generation section of docs/customization.md with the restclient example and a pointer to .openapi-generator-ignore, and a cross-reference from docs/global-properties.md. Correct the Javadoc on CodeGenMojo#supportingFilesToGenerate, which described modelsToGenerate instead, and add the same caveat to the Maven plugin README. Add two tests to JavaClientCodegenTest: one reproducing the reported configuration, where setting the supportingFiles global property to ApiClient.java emits ApiClient.java while skipping the three companions it references, and one pinning the premise the issue assumed had regressed, that a default restclient run emits all four files side by side. No generator behaviour change, so samples are untouched.
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The note added under the global-properties table applied the "a newer generator version adds files an older list does not mention" warning to supportingFiles, models and apis alike. Only supportingFiles behaves that way: its names are template/supporting-file names owned by the generator, so a generator upgrade can introduce one. The models and apis lists filter names that come from the user's OpenAPI document, which a generator upgrade does not change. State the allow-list semantics for all three, then scope the upgrade warning to supportingFiles, and reword the sentence so it parses.
|
Thanks — both addressed.
One open question from this round's internal review, flagging rather than guessing: the sentence now reads "when one of them is set, only the names it lists are generated" — worth confirming the bare-flag form (e.g. |
Nicklas2751
left a comment
There was a problem hiding this comment.
@brbousnguar Thank you for your contribution! Good work. I have no complaints. @wing328 I suggest merging the PR like this. :)
Closes #22238.
What is actually going on
#22238 reports that the
restclientlibrary generates anApiClientwhichreferences
ServerConfiguration/ServerVariablewithout those classes beingavailable, and suggests adding imports. Two things about that:
ApiClient(invokerPackage), so animport would not help — and would not compile.
[java] Support templated servers #4998:
JavaClientCodegen.java:572-573addsServerConfiguration.mustacheand
ServerVariable.mustache, and:764addsExceptionProvider.mustachefor
restclient. A default run emits all of them next toApiClient.java.The reporter's build fails because of their own Maven configuration:
That maps to the
supportingFilesglobal property, whichDefaultGeneratortreats as a fixed allow-list:
The list was written before #21699 taught the
restclientApiClientaboutservers, so
ServerConfiguration.java,ServerVariable.javaandExceptionProvider.javaare now skipped while the emittedApiClientstillreferences them. Reproduced with the CLI equivalent on a minimal spec: with
--global-property supportingFiles="ApiClient.java:..."exactly those threefiles are missing from the output; without it, they are all generated and also
listed in
.openapi-generator/FILES. @BobLuursema reached the same conclusionin the issue thread.
So there is no generation bug to fix — but the trap is undocumented in the
places a user actually reads, and the Maven plugin's own Javadoc for the
option describes the wrong thing. This PR fixes that.
Changes
docs/customization.md— new paragraph in Selective generation noting thatan explicit list replaces the full set rather than adding to it, that
supporting files reference one another (with the
restclientApiClientexample), that each skip is logged at
INFOlevel, and that.openapi-generator-ignoreis the upgrade-safe alternative.docs/global-properties.md— one-line cross-reference to that caveat, sincethis is where
supportingFilesis documented as a global property and where auser building the list reads first.
CodeGenMojo.java— the Javadoc onsupportingFilesToGenerateread "A commaseparated list of models to generate. All models is the default.", copy-pasted
from
modelsToGenerate. Corrected, with the same caveat.modules/openapi-generator-maven-plugin/README.md— caveat added to thesupportingFilesToGeneraterow.JavaClientCodegenTest— two tests:testRestClientSupportingFilesAllowListSkipsApiClientCompanions_issue_22238reproduces the reported configuration: with the
supportingFilesglobalproperty set to
ApiClient.java,ApiClient.javais still emitted while theServerConfiguration,ServerVariableandExceptionProviderit referencesare skipped. This pins and documents the footgun itself.
testRestClientDefaultGenerationIncludesCompanionFilespins the premise theissue assumed had regressed — a default
restclientrun emits all four filesside by side — so the registration cannot silently disappear.
No generator behaviour changes, so
samples/is untouched.Testing
All green: the core library reports
Tests run: 5270, Failures: 0, Errors: 0, Skipped: 14withJavaClientCodegenTestat 299/0, the Maven plugin modulereports 27/0, and checkstyle passes with test sources included.
I also checked the default-generation test is meaningful: commenting out the
ServerConfiguration.mustacheregistration inJavaClientCodegenmakes it failwith
File does not exist when it should: .../ServerConfiguration.java.One note for anyone reproducing locally, since it cost me a round: the
cleanisload-bearing. This repo enables the Develocity Maven extension with the local
build cache on, and a cached
target/classesfrom before #24783 still containedthe ~30 mustache templates that commit moved from
cpp-boost-beast-client/tocpp-boost-beast-common/. Maven's resources plugin never deletes removed files,so the template locator resolved the stale copies and three unrelated tests
failed (
cppboostbeast.ModelApiSurfaceTestand bothtemplating.GeneratorTemplateContentLocatorAdditionalDirsTestmethods). Theypass on a clean build. Relatedly,
mvn ... test compilecannot be used here atall: the second lifecycle pass recompiles the antlr4-generated
KotlinLexer/KotlinParseragainst the compile classpath, whereantlr4-runtimeis test-scoped, and it fails before the reactor reaches theMaven plugin module.
PR checklist
(documentation and tests only), so there is nothing to regenerate.
the docs is the
restclientApiClientyou maintain; happy to reword.Summary by cubic
Documents that the
supportingFilesoption is a fixed allow-list, not an addition, so an explicit list skips supporting files a newer generator version adds — which is what broke the build reported in #22238.docs/global-properties.md, and a recommendation to use.openapi-generator-ignoreas the upgrade-safe alternative.CodeGenMojoJavadoc forsupportingFilesToGenerate(previously copy-pasted frommodelsToGenerate) and the corresponding Maven plugin README row.supportingFiles,models, andapisare all allow-lists, but scopes the generator-upgrade warning tosupportingFiles, since only its names are owned by the generator.restclientrun emits all referenced files.No generator behavior changes;
samples/is untouched.Written for commit 608b42c. Summary will update on new commits.